Implement Todo management features and enhance dashboard application#54
Implement Todo management features and enhance dashboard application#54ahargunyllib wants to merge 48 commits intomainfrom
Conversation
…eateNanoIdWithPrefix functions
- Added @trpc/client and @trpc/server at version ^11.16.0 - Added hono at version ^4.10.6 - Added zod at version ^4.3.6
…ployment setup - Add .env.example for environment variable configuration - Create alchemy.run.ts for deployment logic and GitHub PR comments - Add index.html as the entry point for the dashboard - Set up package.json with dependencies and scripts for development - Implement main.tsx for React application entry and routing - Add robots.txt to disallow web crawlers - Generate routeTree.gen.ts for routing structure - Create root route and index route components - Configure TypeScript with tsconfig.json - Set up Vite configuration for development and build processes
- Introduced Skeleton component for loading placeholders. - Added Slider component for adjustable value selection. - Implemented Sonner for toast notifications with customizable icons. - Created Spinner component for loading indicators. - Developed Switch component for toggle functionality. - Added Table component with subcomponents for structured data display. - Implemented Tabs component for tabbed navigation. - Created Textarea component for multi-line text input. - Developed ToggleGroup and Toggle components for grouped toggle functionality. - Added Tooltip components for contextual hints. - Implemented useIsMobile hook for responsive design. - Created utility function for class name merging. - Added global CSS styles for consistent theming and design. - Configured TypeScript for the UI package.
…gle functionality
… for structured error management
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new dashboard app and many workspace packages (api, core, db, kv, logger, ui, types, utils), wires Cloudflare D1 and KV bindings, implements a tRPC/Hono API and services, scaffolds a Tailwind/Vite React dashboard with TanStack Router and tRPC client, and introduces extensive UI components, migrations, CI deploy workflows, and TypeScript project configs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as React Dashboard
participant tRPC as tRPC Server (Hono)
participant Service as TodoService (core)
participant DB as Drizzle/D1
participant KV as KV cache (Cloudflare KV)
Client->>tRPC: createTodo(title)
tRPC->>Service: createTodo(title, ctx.waitUntil)
Service->>DB: insert todo
DB-->>Service: created todo
Service->>KV: deleteAllTodo() via waitUntil (async invalidate)
Service-->>tRPC: success
tRPC-->>Client: mutation result
Client->>tRPC: getAllTodos (query)
tRPC->>Service: getAllTodos()
Service->>KV: getAllTodo()
alt cache hit
KV-->>Service: todos JSON
else cache miss
Service->>DB: getAllTodos()
DB-->>Service: todos
Service->>KV: setAllTodo(todos) via waitUntil
end
Service-->>tRPC: todos
tRPC-->>Client: todos
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Preview DeploymentCommit: |
Preview DeploymentCommit: |
…2 module and bundler resolution
Preview DeploymentCommit: |
Preview DeploymentCommit: |
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
packages/ui/src/components/kbd.tsx-16-24 (1)
16-24:⚠️ Potential issue | 🟡 MinorType mismatch: Props typed as
"div"but renders<kbd>element.
KbdGroupacceptsReact.ComponentProps<"div">but renders a<kbd>element. This inconsistency could cause runtime issues if div-specific props are passed, and misrepresents the component's actual behavior.🔧 Proposed fix
-function KbdGroup({ className, ...props }: React.ComponentProps<"div">) { +function KbdGroup({ className, ...props }: React.ComponentProps<"kbd">) { return ( <kbd className={cn("inline-flex items-center gap-1", className)} data-slot="kbd-group" {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/kbd.tsx` around lines 16 - 24, KbdGroup is typed as React.ComponentProps<"div"> but renders a <kbd>, causing a props mismatch; change the component prop type to React.ComponentProps<"kbd"> (or an appropriate HTMLAttributes type) and update the function signature of KbdGroup to use that type so the spread {...props} matches a <kbd> element; adjust any callers or exported types if necessary and keep the same className/cn usage.packages/ui/src/components/empty.tsx-68-79 (1)
68-79:⚠️ Potential issue | 🟡 MinorType/element mismatch: typed as
<p>but renders<div>.
EmptyDescriptionis typed withReact.ComponentProps<"p">but renders a<div>. This could cause type mismatches when consumers pass<p>-specific props or expect paragraph semantics.🛠️ Suggested fix — use consistent element
Either change the type to match the element:
-function EmptyDescription({ className, ...props }: React.ComponentProps<"p">) { +function EmptyDescription({ className, ...props }: React.ComponentProps<"div">) { return ( <divOr change the element to match the type:
function EmptyDescription({ className, ...props }: React.ComponentProps<"p">) { return ( - <div + <p className={cn( "text-muted-foreground text-xs/relaxed [&>a:hover]:text-primary [&>a]:underline [&>a]:underline-offset-4", className )} data-slot="empty-description" {...props} /> + </p> - </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/empty.tsx` around lines 68 - 79, The component EmptyDescription is typed as React.ComponentProps<"p"> but renders a <div>, causing prop/semantic mismatches; fix by making the element and its props consistent — either change the generic type to React.ComponentProps<"div"> to match the rendered <div> (update the function signature EmptyDescription({ className, ...props }: React.ComponentProps<"p">) accordingly) or change the rendered element to a <p> so the existing React.ComponentProps<"p"> typing remains correct; ensure data-slot, className merging (cn) and spread {...props} are preserved after the change.packages/ui/src/components/tabs.tsx-8-24 (1)
8-24:⚠️ Potential issue | 🟡 MinorPass
orientationprop toTabsPrimitive.Rootfor correct keyboard navigation.The
orientationprop is destructured but only used for thedata-orientationattribute. The underlying@base-ui/reacttabs primitive requires theorientationprop to handle keyboard navigation correctly: left/right arrows for horizontal orientation, up/down for vertical.🛠️ Suggested fix
function Tabs({ className, orientation = "horizontal", ...props }: TabsPrimitive.Root.Props) { return ( <TabsPrimitive.Root className={cn( "group/tabs flex gap-2 data-horizontal:flex-col", className )} data-orientation={orientation} data-slot="tabs" + orientation={orientation} {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/tabs.tsx` around lines 8 - 24, The Tabs component destructures orientation but never forwards it to TabsPrimitive.Root, which breaks keyboard navigation; update the Tabs function to pass the orientation prop into TabsPrimitive.Root (in addition to keeping data-orientation) so the underlying `@base-ui/react` primitive receives the orientation value and can handle left/right vs up/down arrow behavior (refer to the Tabs function and TabsPrimitive.Root usage and the orientation/data-orientation references).packages/ui/src/components/button-group.tsx-64-77 (1)
64-77:⚠️ Potential issue | 🟡 MinorMatch the separator default to the group orientation.
ButtonGroupsupportsorientation="vertical", butButtonGroupSeparatorstill defaults to"vertical". In a vertical group that renders the wrong divider unless every caller remembers to override it manually.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/button-group.tsx` around lines 64 - 77, ButtonGroupSeparator currently hardcodes orientation="vertical", causing mismatched dividers when used in horizontal ButtonGroup; update ButtonGroupSeparator so it does not default to "vertical" and instead inherits/uses the orientation provided by its parent ButtonGroup (i.e., remove the fixed default in ButtonGroupSeparator and accept orientation from props or the ButtonGroup context so the separator automatically matches ButtonGroup's orientation), referencing the ButtonGroupSeparator and ButtonGroup components and the Separator prop usage to locate where to change.apps/dashboard/src/features/todo/components/todo-container.tsx-17-42 (1)
17-42:⚠️ Potential issue | 🟡 MinorAdd loading state handling to improve UX.
The component handles error and empty states but doesn't handle the loading state. During the initial fetch, users see nothing before data arrives. Additionally,
data?.length === 0is ambiguous—it'strueboth when loading (data isundefined) and when there are genuinely no todos.🛠️ Suggested fix - add loading state
export function TodoContainer() { - const { data, error } = useQuery(trpc.todoRouter.getAllTodos.queryOptions()); + const { data, error, isPending } = useQuery(trpc.todoRouter.getAllTodos.queryOptions()); return ( <div className="flex flex-col gap-4 p-4"> <h1 className="font-bold font-heading text-2xl">Todo List</h1> <CreateTodoForm /> {error && ( <Alert variant="destructive"> <AlertTitle>Error</AlertTitle> <AlertDescription>{error.message}</AlertDescription> </Alert> )} - {data?.length === 0 && ( + {isPending && ( + <div className="flex justify-center p-4"> + {/* Consider using a Spinner or Skeleton component */} + <span>Loading...</span> + </div> + )} + {!isPending && data?.length === 0 && ( <Empty> <EmptyHeader> <EmptyTitle>No todos yet</EmptyTitle>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/features/todo/components/todo-container.tsx` around lines 17 - 42, The component TodoContainer currently calls useQuery(trpc.todoRouter.getAllTodos.queryOptions()) but doesn't handle the loading state, and data?.length === 0 is ambiguous during load; update the hook destructure to include isLoading (or isFetching) and render a loading UI (spinner/skeleton/placeholder) while isLoading is true; change the empty-state check to only show when !isLoading && data?.length === 0, and only map over data when !isLoading && data?.length > 0 so todos render after loading completes.apps/dashboard/src/features/todo/components/todo-card.tsx-49-62 (1)
49-62:⚠️ Potential issue | 🟡 MinorStatic ID causes accessibility issues when multiple cards are rendered.
Using
id="isCompleted"on the checkbox creates duplicate IDs when multipleTodoCardcomponents are rendered, breaking the label-input association and failing accessibility requirements. Each checkbox needs a unique ID.🛡️ Proposed fix using unique IDs
+import { useId } from "react"; ... export function TodoCard({ todo }: Props) { + const checkboxId = useId(); const { toggle, isCompleted } = useIsCompleteToggle({ todo }); ... <Checkbox checked={isCompleted} - id="isCompleted" + id={checkboxId} name="isCompleted" onCheckedChange={() => { toggle(); }} /> <Label className="text-muted-foreground text-xs" - htmlFor="isCompleted" + htmlFor={checkboxId} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/features/todo/components/todo-card.tsx` around lines 49 - 62, The Checkbox and Label use a static id "isCompleted" which causes duplicate IDs when multiple TodoCard components render; update TodoCard to generate or accept a unique id (e.g., use React's useId() or derive from a unique prop like todo.id) and replace the static id in the Checkbox and htmlFor in Label with that unique value; ensure the toggle handler and isCompleted prop usage remain unchanged and that the identifier is passed to both Checkbox (id) and Label (htmlFor).apps/dashboard/src/features/todo/components/todo-card.tsx-40-44 (1)
40-44:⚠️ Potential issue | 🟡 MinorInconsistent state usage - title styling uses
todo.isCompletedinstead ofisCompletedfrom hook.The checkbox uses the optimistic
isCompletedstate from the hook (line 50), but the title's strikethrough styling usestodo.isCompleted(line 41). This causes a visual inconsistency where checking the box updates the checkbox immediately but the title styling only updates after the server responds.♻️ Proposed fix for consistent state usage
<CardTitle> - <span className={todo.isCompleted ? "line-through opacity-60" : ""}> + <span className={isCompleted ? "line-through opacity-60" : ""}> {todo.title} </span> </CardTitle>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/features/todo/components/todo-card.tsx` around lines 40 - 44, The title styling is using the stale prop todo.isCompleted instead of the optimistic local state isCompleted from the hook, causing a visual mismatch with the checkbox; update the CardTitle span to use the hook state (isCompleted) for its className/strikethrough logic so the title updates immediately when the checkbox toggles (refer to the isCompleted state returned by the hook and the CardTitle / checkbox usage in todo-card.tsx).packages/api/src/context.ts-7-20 (1)
7-20:⚠️ Potential issue | 🟡 MinorRemove unused
fetchCreateContextFnOptionsproperty fromCreateContextOptionstype.The
fetchCreateContextFnOptionsproperty is passed at the call site but never destructured or used in the function body. Remove it along with its import.♻️ Proposed fix
-import type { FetchCreateContextFnOptions } from "@trpc/server/adapters/fetch"; type CreateContextOptions = { env: { db: D1Database; }; - fetchCreateContextFnOptions: FetchCreateContextFnOptions; logger: LoggerType; requestId: string; };Also remove from the call site in
apps/api/src/index.ts:return createContext({ env: { db: c.env.DB, }, - fetchCreateContextFnOptions: opts, logger: customLogger, requestId, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/context.ts` around lines 7 - 20, The CreateContextOptions type declares an unused fetchCreateContextFnOptions property and its import; remove the fetchCreateContextFnOptions field from the CreateContextOptions type and delete the corresponding import, then update callers (e.g., where createContext is invoked) to stop passing fetchCreateContextFnOptions so createContext({ env, logger, requestId }) matches its signature; confirm no other references to fetchCreateContextFnOptions remain in the module.packages/logger/src/logger.ts-98-107 (1)
98-107:⚠️ Potential issue | 🟡 MinorSensitive key matching is case-sensitive and non-recursive.
Two potential gaps in the redaction logic:
- Case sensitivity:
apikeywon't matchapiKeyin the sensitive list. Consider case-insensitive matching.- Nested objects: Fields like
{ data: { password: "secret" } }won't have nested sensitive values redacted.🛡️ Proposed fix for case-insensitive matching
for (const key in fields) { + const lowerKey = key.toLowerCase(); if ( - SENSITIVE_KEY.some((sensitiveKey) => key.includes(sensitiveKey)) && + SENSITIVE_KEY.some((sensitiveKey) => lowerKey.includes(sensitiveKey.toLowerCase())) && typeof fields[key] === "string" ) { normalizedFields[key] = "REDACTED"; } else { normalizedFields[key] = fields[key]; } }For recursive redaction, consider implementing a deep traversal if nested sensitive data is a concern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/logger/src/logger.ts` around lines 98 - 107, The current redaction loop using SENSITIVE_KEY, fields and normalizedFields is case-sensitive and non-recursive; update the logic to perform case-insensitive matching (compare key.toLowerCase() against each sensitiveKey.toLowerCase()) and implement a deep traversal helper used inside the loop that walks nested objects/arrays and replaces any string value whose property name matches a sensitive key with "REDACTED" (preserving non-string values as-is); ensure the helper handles objects and arrays, avoids prototype pollution by checking Object.prototype, and use it where normalizedFields[key] is assigned so both top-level and nested sensitive values are redacted.packages/ui/src/components/combobox.tsx-284-301 (1)
284-301:⚠️ Potential issue | 🟡 Minor
ComboboxClearis not exported, butComboboxTriggeris—consider exporting for consistency.
ComboboxClear(line 40) is used internally byComboboxInputbut is not included in the exports list. However,ComboboxTriggerfollows the same pattern and IS exported. Other components in the codebase (e.g.,SelectScrollUpButton,SelectScrollDownButtonin select.tsx) also export similar internal helper components. Either exportComboboxClearfor API consistency, or add a comment explaining why it's intentionally kept internal-only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/combobox.tsx` around lines 284 - 301, Export consistency: include ComboboxClear in the module exports or document why it should remain internal. Update the export block to add ComboboxClear alongside ComboboxTrigger (so external consumers can use the helper), and ensure ComboboxInput still imports/uses ComboboxClear unchanged; alternatively, if you intend it to remain private, add a clear comment near the ComboboxClear declaration explaining it's intentionally internal-only to prevent exporting. Reference the symbols ComboboxClear, ComboboxTrigger, and ComboboxInput when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/alchemy.run.ts`:
- Line 19: The package is referencing migrations via migrationsDir
("./node_modules/@realm/db/migrations") but the `@realm/db` package.json doesn't
publish that folder; update the `@realm/db` package.json to include the migrations
directory (e.g., add a "files" entry that includes "migrations" or adjust
"exports" to expose the migrations path) or change the consuming code that sets
migrationsDir to resolve the migrations path at build time (e.g., point to a
compiled/packaged path or bundled asset) so the migrations are available in
deployed environments; update either the package.json in the `@realm/db` package
or the usage of migrationsDir to fix the missing migrations.
In `@apps/api/src/index.ts`:
- Around line 17-25: The CORS origin is hardcoded to "http://localhost:5173" in
the app.use("/*", cors(...)) call which will break non-local deployments; update
the cors configuration's origin option to read from an environment variable
(e.g. process.env.CORS_ORIGIN or a comma-separated ALLOWED_ORIGINS) and
implement logic in the same file to allow either a single origin, a whitelist
array, or enable a safe wildcard in development only, ensuring the existing
allowMethods, allowHeaders and credentials settings remain; locate the
app.use("/*", cors(...)) invocation and replace the static origin with the
environment-driven resolver so production domains are permitted.
In `@apps/dashboard/package.json`:
- Around line 15-16: The three TanStack packages in
package.json—@tanstack/react-router, `@tanstack/react-router-devtools`, and
`@tanstack/router-plugin`—use mismatched minor versions; update each dependency to
the same release version (pick one stable matching version, e.g., 1.168.10 or
the latest matching trio) so all three share identical semver
(major.minor.patch), ensuring compatibility; change the version strings for
`@tanstack/react-router`, `@tanstack/react-router-devtools`, and
`@tanstack/router-plugin` to the chosen identical version and run install to
verify no peer conflicts.
In `@apps/dashboard/src/features/todo/hooks/use-is-complete-toggle.ts`:
- Around line 40-56: The onError callback will not run when using
mutation.mutateAsync because errors are thrown; wrap the await
mutation.mutateAsync call in a try/catch (or switch to mutation.mutate) so
failures rollback state and show the toast: on success set
lastIsCompleted.current and call
queryClient.invalidateQueries(trpc.todoRouter.getAllTodos.queryKey()), and in
the catch restore setIsCompleted(lastIsCompleted.current) and call
toast.error("Error updating todo", { description: "Please try again." });
reference mutation.mutateAsync, lastIsCompleted.current, setIsCompleted,
queryClient.invalidateQueries, and trpc.todoRouter.getAllTodos.queryKey() to
locate the changes.
In `@apps/dashboard/tsconfig.json`:
- Around line 4-10: The tsconfig's compilerOptions.types currently overwrites
the base config and removes Node typings, causing type errors for files like
alchemy.run.ts that use process.env; update the compilerOptions.types array (the
"types" entry in this tsconfig.json) to include "node" alongside "vite/client"
(e.g., add "node" to the list) or alternatively place alchemy.run.ts under a
separate tsconfig that includes Node types so Node globals are available.
In `@packages/core/src/errors/index.ts`:
- Around line 1-6: Remove this re-exporting index barrel and stop exporting
AppError, ErrorCode, ErrorCodeType, and ErrorMetadata from this file; instead
delete this file (or stop publishing it) and update all import sites that
currently import from the barrel to import directly from "./app-error"
(referencing the exported symbols AppError, ErrorCode, ErrorCode as
ErrorCodeType, and ErrorMetadata); ensure build/tests still pass after replacing
any "from '.../errors'" imports with "from '.../errors/app-error'".
In `@packages/core/src/index.ts`:
- Around line 1-8: This file acts as a barrel re-exporting AppError, ErrorCode,
ErrorCodeType, ErrorMetadata, createTodoService, TodoService and all types,
which violates the repo rule "Avoid barrel files"; either remove this
package-level index.ts and update consumers to import directly from the original
modules (e.g., import { AppError } from "./errors" and import {
createTodoService, TodoService } from "./services"), or—if a package entrypoint
is mandatory—add a documented exception to the coding guidelines and narrow the
exports to only the required public API (avoid export type *), ensuring the
unique symbols (AppError, ErrorCode, createTodoService, TodoService,
ErrorCodeType, ErrorMetadata) are exported only from their originating modules.
In `@packages/db/migrations/meta/0000_snapshot.json`:
- Around line 24-31: The migration uses an invalid SQLite boolean default for
the column is_completed; change the DEFAULT from false to 0 in the migration SQL
(update the CREATE TABLE/column definition in 0000_futuristic_sentinels.sql to
use `DEFAULT 0 NOT NULL` for is_completed) and also update the migration
snapshot entry for is_completed (packages/db/migrations/meta/0000_snapshot.json)
to use an integer default of 0 instead of false so the JSON and SQL agree with
valid SQLite syntax.
In `@packages/db/src/schemas/todo.ts`:
- Around line 12-15: The updatedAt column default only applies on INSERT so
updateTodo must explicitly refresh it; modify the updateTodo implementation that
currently calls .set(todo) (where todo is Partial<InsertTodo>) to override/merge
in an updatedAt value (e.g., set({...todo, updatedAt: new
Date().toISOString()})) before calling .set, ensuring updatedAt is always
updated on UPDATE operations.
In `@packages/ui/src/components/button-group.tsx`:
- Around line 25-30: The file uses React.ComponentProps in the ButtonGroup
signature (and elsewhere) without importing React, causing type-check failures;
import the React types and update usages by adding a type-only import (for
example: import type { ComponentProps } from "react") and replace
React.ComponentProps<"fieldset"> with ComponentProps<"fieldset"> (also replace
any other React.ComponentProps occurrences), keeping the VariantProps<typeof
buttonGroupVariants> part unchanged so ButtonGroup and buttonGroupVariants
continue to type-check correctly.
In `@packages/ui/src/components/empty.tsx`:
- Around line 1-6: The file uses React.ComponentProps in the Empty component but
never imports React; add an import for React at the top of the file (e.g.,
import React from "react"; or import type React from "react"; depending on your
TSX/JSX settings) so that React.ComponentProps resolves and the Empty({
className, ...props }: React.ComponentProps<"div">) component type-checks and
compiles.
In `@packages/ui/src/components/sidebar.tsx`:
- Around line 424-445: The default renderers for button-based components
(SidebarGroupAction, SidebarMenuButton, SidebarMenuAction) omit an explicit
type, which makes them act as submit buttons inside forms; update each
mergeProps<"button"> default props object to include type: "button" (i.e., add
type: "button" alongside className in the props passed to mergeProps in the
SidebarGroupAction, SidebarMenuButton, and SidebarMenuAction implementations) so
the buttons do not submit surrounding forms by default.
- Around line 1-3: Remove the file-wide suppression for noDocumentCookie and
stop assigning directly to document.cookie; instead implement or import a small
setCookie utility and replace the direct document.cookie writes (the occurrences
referenced around lines 81-94) with calls like setCookie(name, value, {
path:'/', secure:true, sameSite:'Lax', maxAge:... }) that properly encodes
name/value and explicitly sets attributes; remove the /** biome-ignore-all
lint/suspicious/noDocumentCookie */ header and update the component/provider
code to use the setCookie helper wherever document.cookie is currently written.
- Around line 287-307: The SidebarRail button component currently spreads
{...props} after assigning onClick={toggleSidebar}, lacks an explicit type, and
sets tabIndex={-1}, which allows callers to override the toggle, can make it
submit forms, and removes it from keyboard navigation; update SidebarRail so
that onClick={toggleSidebar} cannot be overwritten by spreading props (apply
{...props} before the explicit onClick), add an explicit type="button"
attribute, and restore focusability by removing tabIndex={-1} (or set
tabIndex={0} if intentional) to make the control accessible and non-submitting;
ensure you target the SidebarRail function and the toggleSidebar usage when
making these changes.
- Around line 159-167: The Sidebar component destructures dir and className but
only forwards them in the mobile SheetContent branch, causing desktop and
collapsible="none" branches to drop caller-supplied directionality and styling;
update Sidebar (and the alternate render branches around the
SheetContent/SheetTrigger/SheetClose and the non-mobile div render paths) to
always spread remaining props and explicitly pass dir and className to the root
element or wrapper used in each branch (e.g., the outer div, SheetContent, or
sidebar container) so dir and className are consistently applied regardless of
collapsible mode or breakpoint.
- Around line 104-117: The global keydown handler in useEffect (handleKeyDown)
currently toggles the sidebar on SIDEBAR_KEYBOARD_SHORTCUT regardless of focus;
update handleKeyDown to first detect the event target (event.target or
event.composedPath()[0]) and return early if the focused element is an input,
textarea, select, or any element with contentEditable=true (and also if
event.defaultPrevented is true), otherwise proceed to preventDefault() and call
toggleSidebar(); keep the listener registration in useEffect as-is but ensure
the editable-focus check prevents hijacking native/editor shortcuts.
In `@packages/ui/src/components/theme-provider.tsx`:
- Around line 87-94: The useState initializer in theme-provider.tsx reads
localStorage directly which breaks SSR; update the initializer for the Theme
state and the getSystemTheme function to guard all window/localStorage access
behind a runtime check (e.g., typeof window !== 'undefined') so they only call
localStorage.getItem(storageKey) and window.matchMedia(...) in the browser.
Specifically modify the Theme state initializer that currently uses
isTheme(storedTheme) to first ensure window exists before accessing
localStorage, and update getSystemTheme to return defaultTheme (or a safe
fallback) when window or matchMedia is unavailable; keep using the existing
helpers isTheme, storageKey and defaultTheme names so the changes are localized.
---
Minor comments:
In `@apps/dashboard/src/features/todo/components/todo-card.tsx`:
- Around line 49-62: The Checkbox and Label use a static id "isCompleted" which
causes duplicate IDs when multiple TodoCard components render; update TodoCard
to generate or accept a unique id (e.g., use React's useId() or derive from a
unique prop like todo.id) and replace the static id in the Checkbox and htmlFor
in Label with that unique value; ensure the toggle handler and isCompleted prop
usage remain unchanged and that the identifier is passed to both Checkbox (id)
and Label (htmlFor).
- Around line 40-44: The title styling is using the stale prop todo.isCompleted
instead of the optimistic local state isCompleted from the hook, causing a
visual mismatch with the checkbox; update the CardTitle span to use the hook
state (isCompleted) for its className/strikethrough logic so the title updates
immediately when the checkbox toggles (refer to the isCompleted state returned
by the hook and the CardTitle / checkbox usage in todo-card.tsx).
In `@apps/dashboard/src/features/todo/components/todo-container.tsx`:
- Around line 17-42: The component TodoContainer currently calls
useQuery(trpc.todoRouter.getAllTodos.queryOptions()) but doesn't handle the
loading state, and data?.length === 0 is ambiguous during load; update the hook
destructure to include isLoading (or isFetching) and render a loading UI
(spinner/skeleton/placeholder) while isLoading is true; change the empty-state
check to only show when !isLoading && data?.length === 0, and only map over data
when !isLoading && data?.length > 0 so todos render after loading completes.
In `@packages/api/src/context.ts`:
- Around line 7-20: The CreateContextOptions type declares an unused
fetchCreateContextFnOptions property and its import; remove the
fetchCreateContextFnOptions field from the CreateContextOptions type and delete
the corresponding import, then update callers (e.g., where createContext is
invoked) to stop passing fetchCreateContextFnOptions so createContext({ env,
logger, requestId }) matches its signature; confirm no other references to
fetchCreateContextFnOptions remain in the module.
In `@packages/logger/src/logger.ts`:
- Around line 98-107: The current redaction loop using SENSITIVE_KEY, fields and
normalizedFields is case-sensitive and non-recursive; update the logic to
perform case-insensitive matching (compare key.toLowerCase() against each
sensitiveKey.toLowerCase()) and implement a deep traversal helper used inside
the loop that walks nested objects/arrays and replaces any string value whose
property name matches a sensitive key with "REDACTED" (preserving non-string
values as-is); ensure the helper handles objects and arrays, avoids prototype
pollution by checking Object.prototype, and use it where normalizedFields[key]
is assigned so both top-level and nested sensitive values are redacted.
In `@packages/ui/src/components/button-group.tsx`:
- Around line 64-77: ButtonGroupSeparator currently hardcodes
orientation="vertical", causing mismatched dividers when used in horizontal
ButtonGroup; update ButtonGroupSeparator so it does not default to "vertical"
and instead inherits/uses the orientation provided by its parent ButtonGroup
(i.e., remove the fixed default in ButtonGroupSeparator and accept orientation
from props or the ButtonGroup context so the separator automatically matches
ButtonGroup's orientation), referencing the ButtonGroupSeparator and ButtonGroup
components and the Separator prop usage to locate where to change.
In `@packages/ui/src/components/combobox.tsx`:
- Around line 284-301: Export consistency: include ComboboxClear in the module
exports or document why it should remain internal. Update the export block to
add ComboboxClear alongside ComboboxTrigger (so external consumers can use the
helper), and ensure ComboboxInput still imports/uses ComboboxClear unchanged;
alternatively, if you intend it to remain private, add a clear comment near the
ComboboxClear declaration explaining it's intentionally internal-only to prevent
exporting. Reference the symbols ComboboxClear, ComboboxTrigger, and
ComboboxInput when making the change.
In `@packages/ui/src/components/empty.tsx`:
- Around line 68-79: The component EmptyDescription is typed as
React.ComponentProps<"p"> but renders a <div>, causing prop/semantic mismatches;
fix by making the element and its props consistent — either change the generic
type to React.ComponentProps<"div"> to match the rendered <div> (update the
function signature EmptyDescription({ className, ...props }:
React.ComponentProps<"p">) accordingly) or change the rendered element to a <p>
so the existing React.ComponentProps<"p"> typing remains correct; ensure
data-slot, className merging (cn) and spread {...props} are preserved after the
change.
In `@packages/ui/src/components/kbd.tsx`:
- Around line 16-24: KbdGroup is typed as React.ComponentProps<"div"> but
renders a <kbd>, causing a props mismatch; change the component prop type to
React.ComponentProps<"kbd"> (or an appropriate HTMLAttributes type) and update
the function signature of KbdGroup to use that type so the spread {...props}
matches a <kbd> element; adjust any callers or exported types if necessary and
keep the same className/cn usage.
In `@packages/ui/src/components/tabs.tsx`:
- Around line 8-24: The Tabs component destructures orientation but never
forwards it to TabsPrimitive.Root, which breaks keyboard navigation; update the
Tabs function to pass the orientation prop into TabsPrimitive.Root (in addition
to keeping data-orientation) so the underlying `@base-ui/react` primitive receives
the orientation value and can handle left/right vs up/down arrow behavior (refer
to the Tabs function and TabsPrimitive.Root usage and the
orientation/data-orientation references).
---
Nitpick comments:
In `@apps/api/package.json`:
- Around line 10-16: The CI deploy is failing with a 401 Unauthorized (code
10000) for the Alchemy/Cloudflare deploy, which is an auth/secret issue rather
than a dependency problem; verify that the Cloudflare API token/credentials used
by the `alchemy` CLI (the dependency named "alchemy" and the deploy invocation
`alchemy deploy --stage pr-54`) are correctly configured in the repo/org CI
secrets and that the token has the required permissions for PR-stage deploys,
updating the repository or organization secrets and any environment variable
names referenced in the CI workflow so the `alchemy` step receives valid
credentials.
In `@apps/dashboard/index.html`:
- Line 11: The module entry script currently uses the async attribute on the tag
referencing "/src/main.tsx"; remove the async attribute from the <script
type="module" src="/src/main.tsx"> tag so the module script uses its default
deferred behavior and preserves deterministic boot order as other scripts are
added.
In `@apps/dashboard/src/features/todo/components/create-todo-form.tsx`:
- Around line 27-36: Input uses only a placeholder which is insufficient for
screen readers; add a visually hidden <Label> associated with the input (use
htmlFor={field.name} / id={field.name}) before the <Input> in
create-todo-form.tsx, give the label text like "New todo" and apply the
project's screen-reader-only class (e.g., "sr-only") so it is announced to
assistive tech but not visually shown; keep existing props (aria-invalid, id,
name, onBlur, onChange, value) on the Input and ensure the label text matches
the placeholder for clarity.
In `@apps/dashboard/src/features/todo/hooks/use-is-complete-toggle.ts`:
- Line 16: The timeoutRef in use-is-complete-toggle.ts is typed as
NodeJS.Timeout which is Node-specific; change its type to use ReturnType<typeof
setTimeout> (e.g., const timeoutRef = useRef<ReturnType<typeof setTimeout> |
null>(null)) so the hook (useIsCompleteToggle / timeoutRef) works correctly in
both browser and Node environments.
- Around line 15-18: The local isCompleted state is only initialized from
initialIsCompleted and can become stale when the parent updates the prop; add a
useEffect that watches initialIsCompleted and, when it changes, updates
latestIsCompletedRef.current and lastIsCompleted.current and calls
setIsCompleted(initialIsCompleted) only if no debounce is in progress (check
timeoutRef.current), otherwise just update the refs so the pending debounce will
use the new value; reference useState (isCompleted, setIsCompleted), timeoutRef,
latestIsCompletedRef, and lastIsCompleted when implementing this sync.
In `@apps/dashboard/src/routes/__root.tsx`:
- Around line 31-38: RootComponent currently renders TanStackRouterDevtools
unconditionally which increases bundle size and exposes internals in production;
update RootComponent to only include TanStackRouterDevtools in development by
conditionally rendering it (e.g., process.env.NODE_ENV === 'development' or an
isDev flag) or lazy-load it with React.lazy/Suspense so it isn’t part of the
production bundle; reference the TanStackRouterDevtools import and the
RootComponent function to guard rendering, and ensure HeadContent and Outlet
remain unaffected.
In `@apps/dashboard/src/shared/lib/query-client.ts`:
- Around line 7-8: Replace the inline magic-number durations used for query
cache settings by extracting them into named constants (e.g., STALE_TIME_MS and
GC_TIME_MS or CACHE_STALE_TIME and CACHE_GC_TIME) and use those constants where
staleTime and gcTime are set; update the file's top-level scope (near the query
client creation) so other query settings can reuse these descriptive constants
and make tuning easier.
In `@apps/dashboard/src/shared/lib/trpc.ts`:
- Line 10: The API URL construction can produce double slashes if
VITE_PUBLIC_API_URL ends with a slash; replace the template string assignment
for the url property with a robust join using the URL constructor: build a base
from import.meta.env.VITE_PUBLIC_API_URL || "http://localhost:3000" and use new
URL("/trpc", base).toString() (or .href) when setting the url (the url property
in this file), so the final endpoint never contains accidental double slashes.
In `@packages/api/src/routers/todo.ts`:
- Around line 22-35: The input schema for updateTodo permits no-op updates
(calling updateTodo with only id), so modify the z.object passed to updateTodo's
.input to enforce that at least one of title or isCompleted is provided: add a
.refine(...) on the object returned by z.object({ id, title, isCompleted }) that
checks (data.title !== undefined || data.isCompleted !== undefined) and supply a
clear error message like "At least one of title or isCompleted is required";
keep the rest of the mutation (mutation handler calling
ctx.services.todo.updateTodo) unchanged so types still align with the service
call.
- Around line 10-20: The createTodo mutation (publicProcedure named createTodo)
currently awaits ctx.services.todo.createTodo(...) but doesn't return the
created record, preventing clients from getting server-generated fields; modify
the mutation to return the result of ctx.services.todo.createTodo(...) (i.e.,
remove the void await-only pattern and return the created todo object), and
ensure the mutation's TypeScript return type aligns with the service's return
(adjust input/output typing if needed) so clients receive the created todo with
id/createdAt/updatedAt.
In `@packages/core/src/errors/app-error.ts`:
- Around line 18-40: The AppError class should set a meaningful error name for
better identification; in the AppError constructor assign this.name (e.g.,
"AppError" or a dynamic name derived from the code) after calling super so stack
traces show the class name; update the constructor in AppError to set this.name
= "AppError" (or this.name = `AppError:${code}` if you prefer dynamic naming)
alongside existing assignments to code, details, and metadata.
In `@packages/core/src/services/index.ts`:
- Line 1: Remove the barrel re-export in this module: delete the line exporting
createTodoService and TodoService from "./todo" so consumers import directly
from the concrete module; update any callers that import from this services
index to instead import { createTodoService, type TodoService } from "./todo"
(or the appropriate relative path) and ensure no other code relies on this
index.ts re-export.
In `@packages/core/src/services/todo.ts`:
- Line 19: getAllTodos currently returns await todoQueries.getAllTodos()
directly and can throw unwrapped errors; update it to follow the same pattern as
createTodo/updateTodo/deleteTodo by wrapping the call in a try/catch (or use the
existing tryCatch helper) and rethrowing a consistent AppError on failure.
Specifically, in the getAllTodos function catch any error from
todoQueries.getAllTodos() and throw new AppError with a clear message (e.g.,
"Failed to get all todos") and include the original error for context so error
handling is consistent across the service.
- Around line 21-27: The newTodo construction uses two separate new
Date().toISOString() calls for createdAt and updatedAt which can differ by a
millisecond; capture a single timestamp variable (e.g., const now = new
Date().toISOString()) before creating newTodo and use that same now value for
both createdAt and updatedAt when building the Todo (referencing newTodo, Todo,
createNanoId, createdAt, updatedAt).
In `@packages/core/src/types/index.ts`:
- Line 1: Remove the barrel re-export line `export type * from "./todo";` from
the types index file and stop using this index barrel; instead update all
imports that currently reference the types barrel to import directly from the
concrete module (`./todo`) where the types are defined (i.e., import the
specific types from the todo module), and delete the empty/unused index file if
nothing else remains in it.
In `@packages/db/migrations/meta/0000_snapshot.json`:
- Line 49: The migration snapshot currently has an empty "indexes" object; add a
new index entry (e.g., "idx_todos_is_completed") under the "indexes" key that
targets the "is_completed" column so filtered queries on completion status are
accelerated; ensure the index definition follows the existing snapshot schema
(name, columns: ["is_completed"], and any required metadata such as
unique/primary flags set to false) so the migration system picks it up.
In `@packages/db/src/queries/index.ts`:
- Line 1: Remove the re-export-only barrel that exports createTodoQueries and
TodoQueries: delete the index file that only contains "export {
createTodoQueries, type TodoQueries } from './todo'"; then update all project
imports that currently import those symbols from the barrel to import directly
from "./todo" (e.g., change imports to "import { createTodoQueries, type
TodoQueries } from './todo'") and run the build/tests to ensure no remaining
references to the removed barrel.
In `@packages/db/src/queries/todo.ts`:
- Around line 18-26: The updateTodo and deleteTodo functions currently perform
db operations silently; change them to detect no-op and return or throw: for
updateTodo, capture Drizzle's result (use .returning(...) or the driver's
affectedRows/rowCount from the update call) and return the affected row count or
throw an error if count is 0; do the same for deleteTodo (use .returning() or
the delete result's rowCount) so callers can detect non-existent IDs and handle
them appropriately. Ensure references to updateTodo and deleteTodo are updated
to return the count or raise a NotFound-style error when zero rows are affected.
In `@packages/logger/src/index.ts`:
- Line 1: The current barrel export (exporting createLogger, Logger, LoggerType)
should be removed and consumers updated to import directly from the
implementation module; delete or stop publishing this re-exporting index and
ensure all imports reference the concrete module that defines
createLogger/Logger/LoggerType (the "./logger" module) so the package follows
the rule against index barrel files. Update package entry/exports if necessary
to point to the implementation file instead of re-exporting via an index.
In `@packages/logger/src/logger.ts`:
- Around line 75-90: The SENSITIVE_KEY array is too broad and contains redundant
entries; remove the generic "key" (to avoid false positives like
primaryKey/foreignKey), drop the duplicate "Auth" (case-insensitive matching
already covers "auth"), and remove "authorization" since "auth" subsumes it; in
SENSITIVE_KEY (the constant array) keep only specific tokens like "apiKey",
"api_key", "access_token", "refresh_token", "client_id", "client_secret",
"password", "token", "secret", "credential", "auth", "authorization" should be
removed and "key" replaced by more specific entries if needed or handled via
stricter matching rules (e.g., exact/key-name matching or word-boundary checks)
in the sanitization logic.
In `@packages/ui/src/components/chart.tsx`:
- Around line 97-118: The generated CSS string for the style tag (built from
THEMES, id, and colorConfig) can be hardened by validating and escaping config
keys and color values before interpolation: add a helper (e.g.,
sanitizeCssIdentifier and validateColor) and run it on each key from colorConfig
and each itemConfig.color / itemConfig.theme[...] to allow only safe identifier
characters (e.g., [A-Za-z0-9_-]) and whitelist color formats (hex, rgb(a),
hsl(a), or known names); reject or fallback to a safe default when validation
fails, then use the sanitized values when building the string for
dangerouslySetInnerHTML in the Chart component so only validated content is
injected.
In `@packages/ui/src/components/combobox.tsx`:
- Around line 219-234: ComboboxChips currently uses a redundant prop type
intersection; replace the type signature React.ComponentPropsWithRef<typeof
ComboboxPrimitive.Chips> & ComboboxPrimitive.Chips.Props with just
ComboboxPrimitive.Chips.Props to match the other components and avoid
duplication. Update the ComboboxChips function parameter typing (the
destructured props line) to use ComboboxPrimitive.Chips.Props only, keep the
rest of the implementation unchanged, and run type-checks to ensure ref support
is preserved via the .Props type on ComboboxPrimitive.Chips.
In `@packages/ui/src/components/context-menu.tsx`:
- Around line 145-156: The ContextMenuSubContent component duplicates many style
utilities already applied by ContextMenuContent; update ContextMenuSubContent by
removing redundant class tokens (e.g., before:-z-1, bg-popover/70,
before:backdrop-blur-2xl/backdrop-saturate-150 and the repeated
**:data-[slot$=...] / **:data-[variant=destructive] selectors) from the
className passed into ContextMenuContent so it only supplies
unique/sub-content-specific classes (keep data-slot="context-menu-sub-content",
side="right" and any modifiers that are truly different), ensuring the component
delegates common styling to ContextMenuContent and only overrides or adds what’s
necessary.
In `@packages/ui/src/components/field.tsx`:
- Line 1: Replace the file-level "biome-ignore-all
lint/suspicious/noArrayIndexKey" directive with a localized ignore comment
placed directly above the array-index key usage in the FieldError component;
remove the top-of-file ignore, locate the FieldError function/component (symbol:
FieldError) and add a single-line "biome-ignore" (targeting
lint/suspicious/noArrayIndexKey) immediately above the specific line where the
array index is used as a key so only that instance is suppressed.
- Around line 119-130: FieldTitle currently uses the same data-slot value as
FieldLabel ("field-label"), which can cause CSS selector collisions; update the
data-slot in the FieldTitle component to a unique value (e.g., "field-title")
and then audit/update any related CSS/slot-based selectors that rely on
data-slot to reference the new "field-title" identifier instead of "field-label"
so styles remain correct; check the FieldTitle function and any CSS modules or
styled components that target data-slot="field-label" and adjust accordingly.
- Around line 194-209: The optional chaining on uniqueErrors is unnecessary
because uniqueErrors is always an array created by the spread/map expression;
update the conditional in the render logic (referencing uniqueErrors) to use a
plain property access (uniqueErrors.length === 1) instead of
uniqueErrors?.length and remove any other redundant optional chaining around
uniqueErrors to simplify the code and avoid misleading null-safety checks.
In `@packages/ui/src/components/input-group.tsx`:
- Around line 1-2: The click-only interaction on InputGroupAddon is
inaccessible: update the InputGroupAddon implementation (or the component
containing the onClick handler) to support keyboard activation by either using a
semantic interactive element (replace the non-interactive wrapper with a
<button> or similar) or adding role="button", tabIndex={0}, and an onKeyDown
handler that triggers the same logic on Enter and Space (preventDefault for
Space so it doesn't scroll), and ensure the handler focuses the associated input
ref the same way as the onClick; remove the biome-ignore-a11y directives once
keyboard support is added and consider adding an appropriate
aria-label/aria-describedby if the action isn't obvious.
- Around line 46-67: Replace the semantic misuse of <fieldset> in
InputGroupAddon with a neutral container: change the component signature to
accept React.ComponentProps<"div"> (instead of "fieldset") and render a <div>
(keeping className={cn(inputGroupAddonVariants({ align }), className)},
data-align, data-slot, the onClick handler, and {...props}); if grouping
semantics are still required, add an explicit role (e.g., role="group") or
aria-label rather than using a fieldset; ensure inputGroupAddonVariants, align,
className and the onClick focus logic remain unchanged.
In `@packages/ui/src/components/item.tsx`:
- Around line 10-22: ItemGroup renders a container with role="list" but Item
does not supply role="listitem", so assistive tech won't associate items
correctly; update the Item component to accept and forward role (or explicitly
set role="listitem" on the rendered element) so when used inside ItemGroup each
item has role="listitem", and ensure the prop is forwarded in the component that
renders the DOM element (reference the Item component and ItemGroup's
role="list" usage).
In `@packages/ui/src/components/label.tsx`:
- Line 1: Remove the file-level "biome-ignore-all" directive in
packages/ui/src/components/label.tsx and replace it with a targeted lint
approach: either delete the suppression entirely so the Label component (Label)
relies on consuming components like FieldLabel (in field.tsx) to provide proper
input associations, or add inline/specific biome rule disables only where
association cannot be established (e.g., use // biome-disable-next-line
lint/a11y/noLabelWithoutControl on the specific usage). If you must keep a
suppression for a documented standalone usage, add a narrow comment explaining
why and limit it to the exact rule and lines rather than the whole file.
In `@packages/ui/src/components/native-select.tsx`:
- Around line 6-29: The component currently consumes a consumer-provided
className on the wrapper div, which confuses callers expecting className to
style the actual <select>; update the props and component so styling is
unambiguous: change NativeSelectProps to Omit<React.ComponentProps<"select">,
"size" | "className"> & { size?: "sm" | "default"; wrapperClassName?: string;
className?: string }; in the NativeSelect function destructure {
wrapperClassName, className, size = "default", ...props } and apply
wrapperClassName to the outer <div> and className to the <select> (using cn to
merge with existing classes), so consumers can pass wrapperClassName for the
wrapper and className for the native select element.
In `@packages/ui/src/components/progress.tsx`:
- Around line 7-26: The Progress component currently always injects its own
<ProgressTrack><ProgressIndicator/></ProgressTrack> after children, preventing
consumers from supplying custom track/indicator; update the Progress function
(the component using ProgressPrimitive.Root, props like className, children,
value) to stop rendering the internal ProgressTrack/ProgressIndicator — only
render children inside ProgressPrimitive.Root (preserving cn("flex flex-wrap
gap-3", className), data-slot, value and {...props}) so consumers must compose
their own <ProgressTrack> and <ProgressIndicator> when needed.
In `@packages/ui/src/components/slider.tsx`:
- Around line 18-26: The fallback for the slider's computed _values (in the
useMemo that reads value, defaultValue, min, max) currently returns [min, max]
when neither prop is provided, producing a two-thumb range slider; change that
fallback to a single-thumb default such as [min] (i.e., return [min] instead of
[min, max]) so consumers get a single-thumb slider by default, and keep the
dependency array [value, defaultValue, min, max] unchanged.
In `@packages/ui/src/components/theme-provider.tsx`:
- Around line 160-172: The setThemeState updater contains a side effect
(localStorage.setItem) which must be removed; instead compute nextTheme outside
the state updater (or get it from the updater return), call
setThemeState(nextTheme) purely, and then persist with
localStorage.setItem(storageKey, nextTheme) in the next line or inside a
useEffect that watches theme/storageKey; references: setThemeState,
getSystemTheme, storageKey, localStorage.setItem, and theme.
- Around line 96-102: The keyboard handler duplicates persistence logic already
in setTheme — instead of calling localStorage.setItem or setThemeState directly
from the handler, compute the next theme value in the handler and call the
existing setTheme(nextTheme) function; remove any direct localStorage.setItem
and setThemeState calls from the keyboard handler so persistence is centralized
in setTheme. Apply the same refactor to the other duplicated block (the similar
handler at lines ~160-172) so both use setTheme to update and persist theme.
In `@packages/ui/src/components/toggle-group.tsx`:
- Around line 74-82: The fallback expressions in the toggle-group component use
|| which treats falsy values (like empty string or 0) as absent; update the
fallback to use nullish coalescing (??) for context.variant and context.size
(and any other context.* || localVar patterns such as context.spacing if
intended) so they only fall back when the context value is null or undefined;
locate the occurrences in the render/props where context.variant || variant and
context.size || size are used (and data-spacing={context.spacing} if analogous),
and replace them with context.variant ?? variant and context.size ?? size (and
context.spacing ?? spacing) to preserve explicit falsy values.
In `@packages/utils/package.json`:
- Around line 18-20: Replace the direct versioned dependency for "nanoid" in
packages/utils's package.json with a catalog reference (use "nanoid":
"catalog:nanoid") so the workspace catalog governs the version, and add an entry
under the pnpm workspace catalog (pnpm-workspace.yaml) mapping nanoid to the
desired version (e.g., ^5.1.7); update the package.json dependency key "nanoid"
and the pnpm-workspace.yaml catalog accordingly.
In `@packages/utils/src/identifier/index.ts`:
- Around line 1-4: This barrel file simply re-exports createNanoId and
createNanoIdWithPrefix from "./nanoid" and is unnecessary; either remove this
intermediary file and update all consumers to import { createNanoId,
createNanoIdWithPrefix } directly from "./nanoid", or move these exports into
the package's primary entry (the main utils index) so consumers can import them
without this extra barrel. Locate references to createNanoId and
createNanoIdWithPrefix in the repo and update their import paths accordingly,
then delete this file (or replace its content with a direct forward in the main
entry) to eliminate the redundant barrel.
In `@packages/utils/src/identifier/nanoid.ts`:
- Around line 24-25: createNanoIdWithPrefix should reuse the existing
createNanoId function instead of calling nanoid directly; update
createNanoIdWithPrefix to call createNanoId(size) when building the suffix
(e.g., `${prefix}_${createNanoId(size)}`) so any future changes in createNanoId
(validation, logging, etc.) automatically apply to prefixed IDs; ensure
imports/exports still match and run tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f0683e3-e4a3-435d-99cb-4b2a6eae1dcd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (116)
.gitignore.vscode/settings.jsonapps/api/alchemy.run.tsapps/api/package.jsonapps/api/src/env.tsapps/api/src/index.tsapps/dashboard/.env.exampleapps/dashboard/alchemy.run.tsapps/dashboard/index.htmlapps/dashboard/package.jsonapps/dashboard/public/robots.txtapps/dashboard/src/features/todo/components/create-todo-form.tsxapps/dashboard/src/features/todo/components/todo-card.tsxapps/dashboard/src/features/todo/components/todo-container.tsxapps/dashboard/src/features/todo/hooks/use-create-todo-form.tsapps/dashboard/src/features/todo/hooks/use-delete-todo-form.tsapps/dashboard/src/features/todo/hooks/use-is-complete-toggle.tsapps/dashboard/src/main.tsxapps/dashboard/src/routeTree.gen.tsapps/dashboard/src/routes/__root.tsxapps/dashboard/src/routes/todo.tsxapps/dashboard/src/shared/lib/query-client.tsapps/dashboard/src/shared/lib/trpc.tsapps/dashboard/tsconfig.jsonapps/dashboard/vite.config.tspackage.jsonpackages/api/package.jsonpackages/api/src/context.tspackages/api/src/index.tspackages/api/src/routers/index.tspackages/api/src/routers/todo.tspackages/api/src/trpc.tspackages/api/tsconfig.jsonpackages/core/package.jsonpackages/core/src/errors/app-error.tspackages/core/src/errors/index.tspackages/core/src/index.tspackages/core/src/services/index.tspackages/core/src/services/todo.tspackages/core/src/types/index.tspackages/core/src/types/todo.tspackages/core/tsconfig.jsonpackages/db/drizzle.config.tspackages/db/migrations/0000_futuristic_sentinels.sqlpackages/db/migrations/meta/0000_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/package.jsonpackages/db/src/db.tspackages/db/src/index.tspackages/db/src/queries/index.tspackages/db/src/queries/todo.tspackages/db/src/schemas/index.tspackages/db/src/schemas/todo.tspackages/db/tsconfig.jsonpackages/logger/package.jsonpackages/logger/src/index.tspackages/logger/src/logger.tspackages/logger/tsconfig.jsonpackages/tsconfig/package.jsonpackages/tsconfig/react.jsonpackages/ui/components.jsonpackages/ui/package.jsonpackages/ui/src/components/alert-dialog.tsxpackages/ui/src/components/alert.tsxpackages/ui/src/components/avatar.tsxpackages/ui/src/components/badge.tsxpackages/ui/src/components/button-group.tsxpackages/ui/src/components/button.tsxpackages/ui/src/components/calendar.tsxpackages/ui/src/components/card.tsxpackages/ui/src/components/chart.tsxpackages/ui/src/components/checkbox.tsxpackages/ui/src/components/combobox.tsxpackages/ui/src/components/command.tsxpackages/ui/src/components/context-menu.tsxpackages/ui/src/components/dialog.tsxpackages/ui/src/components/drawer.tsxpackages/ui/src/components/dropdown-menu.tsxpackages/ui/src/components/empty.tsxpackages/ui/src/components/field.tsxpackages/ui/src/components/hover-card.tsxpackages/ui/src/components/input-group.tsxpackages/ui/src/components/input.tsxpackages/ui/src/components/item.tsxpackages/ui/src/components/kbd.tsxpackages/ui/src/components/label.tsxpackages/ui/src/components/native-select.tsxpackages/ui/src/components/pagination.tsxpackages/ui/src/components/popover.tsxpackages/ui/src/components/progress.tsxpackages/ui/src/components/radio-group.tsxpackages/ui/src/components/select.tsxpackages/ui/src/components/separator.tsxpackages/ui/src/components/sheet.tsxpackages/ui/src/components/sidebar.tsxpackages/ui/src/components/skeleton.tsxpackages/ui/src/components/slider.tsxpackages/ui/src/components/sonner.tsxpackages/ui/src/components/spinner.tsxpackages/ui/src/components/switch.tsxpackages/ui/src/components/table.tsxpackages/ui/src/components/tabs.tsxpackages/ui/src/components/textarea.tsxpackages/ui/src/components/theme-provider.tsxpackages/ui/src/components/toggle-group.tsxpackages/ui/src/components/toggle.tsxpackages/ui/src/components/tooltip.tsxpackages/ui/src/hooks/use-is-mobile.tspackages/ui/src/lib/utils.tspackages/ui/src/styles/globals.csspackages/ui/tsconfig.jsonpackages/utils/package.jsonpackages/utils/src/identifier/index.tspackages/utils/src/identifier/nanoid.tspackages/utils/src/index.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- apps/api/src/env.ts
apps/dashboard/src/features/todo/hooks/use-is-complete-toggle.ts
Outdated
Show resolved
Hide resolved
Preview DeploymentCommit: |
Preview DeploymentCommit: |
… use tryCatch for error handling
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/index.ts`:
- Around line 17-36: The CORS origin callback (the origin: (origin) => { ... }
passed to cors in the app.use call) is using origin.includes and is vulnerable
to spoofing; replace the substring check with proper hostname validation by
parsing the origin (new URL(origin).hostname) and comparing the hostname to
allowedOrigins either by exact equality or by suffix matching like hostname ===
allowedOrigin || hostname.endsWith("." + allowedOrigin) for each allowedOrigin
in the allowedOrigins array, explicitly return the allowed origin string on
match and return null when no match (and handle null/undefined origin inputs
safely).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc6c56b4-5ff9-4a81-8d76-9ba3fd59dfb1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.github/workflows/dashboard-deployment.yamlapps/api/alchemy.run.tsapps/api/src/index.tsapps/dashboard/package.jsonapps/dashboard/src/features/todo/hooks/use-is-complete-toggle.tspackages/api/src/trpc.ts
✅ Files skipped from review due to trivial changes (2)
- apps/dashboard/package.json
- .github/workflows/dashboard-deployment.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dashboard/src/features/todo/hooks/use-is-complete-toggle.ts
- packages/api/src/trpc.ts
- apps/api/alchemy.run.ts
…pe definitions and configuration
…etAllTodo and deleteAllTodo methods
…dding todo operations
…in pnpm-lock.yaml
Preview DeploymentCommit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/types/src/index.ts (1)
1-1: Avoid wildcard barrel re-export from the index.Line 1 uses
export type *, which implicitly broadens the package surface and conflicts with the repo rule forindexfiles. Prefer explicit type exports.♻️ Suggested change
-export type * from "./todo"; +export type { Todo } from "./todo";As per coding guidelines:
**/index.{ts,tsx,js,jsx}: Avoid barrel files (index files that re-export everything).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/types/src/index.ts` at line 1, The index.ts file currently uses a wildcard type re-export ("export type * from \"./todo\""); replace this with explicit named type re-exports by opening ./todo to identify the exact exported type names and re-export them explicitly (e.g., export type { TypeA, TypeB } from "./todo"). Ensure you only re-export the specific types needed (no wildcard) to comply with the index barrel-file rule and keep the package surface minimal.packages/kv/src/operations/index.ts (1)
1-1: Remove the extra barrel file.This file only forwards
./todo, so it adds another import hop without shaping the API. Export the concrete module from the package entrypoint instead.Based on learnings: "Applies to **/index.{ts,tsx,js,jsx} : Avoid barrel files (index files that re-export everything)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kv/src/operations/index.ts` at line 1, Delete the trivial re-export barrel (the file that only re-exports createTodoOperations and TodoOperations) and instead export those symbols directly from the package entrypoint; locate usages/imports that reference the current operations index and update them to import createTodoOperations and TodoOperations from the concrete todo module export instead, then remove the now-unused barrel file to eliminate the extra import hop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kv/src/kv.ts`:
- Around line 20-23: The set wrapper currently forces expirationTtl: 60 even
when callers pass no options; update the set implementation (the set async
function that calls kv.put) to omit expirationTtl entirely unless
options.expirationTtl is explicitly provided (i.e., only include expirationTtl
in the kv.put options when options.expirationTtl !== undefined), or
alternatively implement two clear methods such as setPersistent and setCached
and move the 60-second TTL into setCached, so callers who want indefinite
storage use setPersistent and callers wanting short-lived caching use setCached;
adjust references to set, kv.put, and options.expirationTtl accordingly.
In `@packages/kv/src/operations/todo.ts`:
- Around line 5-6: The getAllTodo KV operation currently returns [] for cache
misses which is indistinguishable from a real cached empty list; change the
TodoOperations signature and implementation for getAllTodo to return
Promise<Todo[] | null> (use null as the cache-miss sentinel) and update any
similar KV methods (the block referenced at lines 15-18) to use the same
nullable pattern; then update the caller in packages/core/src/services/todo.ts
to check cachedTodos !== null (instead of if (cachedTodos)) before falling back
to the DB so a missing/expired KV entry does not short-circuit valid persisted
todos.
---
Nitpick comments:
In `@packages/kv/src/operations/index.ts`:
- Line 1: Delete the trivial re-export barrel (the file that only re-exports
createTodoOperations and TodoOperations) and instead export those symbols
directly from the package entrypoint; locate usages/imports that reference the
current operations index and update them to import createTodoOperations and
TodoOperations from the concrete todo module export instead, then remove the
now-unused barrel file to eliminate the extra import hop.
In `@packages/types/src/index.ts`:
- Line 1: The index.ts file currently uses a wildcard type re-export ("export
type * from \"./todo\""); replace this with explicit named type re-exports by
opening ./todo to identify the exact exported type names and re-export them
explicitly (e.g., export type { TypeA, TypeB } from "./todo"). Ensure you only
re-export the specific types needed (no wildcard) to comply with the index
barrel-file rule and keep the package surface minimal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a9fc061-7b62-44ed-96f9-0742f470cafc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
apps/api/alchemy.run.tsapps/api/package.jsonapps/api/src/index.tspackages/api/package.jsonpackages/api/src/context.tspackages/core/package.jsonpackages/core/src/index.tspackages/core/src/services/todo.tspackages/core/src/types/context.tspackages/core/src/types/index.tspackages/db/src/queries/todo.tspackages/kv/package.jsonpackages/kv/src/index.tspackages/kv/src/keys.tspackages/kv/src/kv.tspackages/kv/src/operations/index.tspackages/kv/src/operations/todo.tspackages/kv/tsconfig.jsonpackages/types/package.jsonpackages/types/src/index.tspackages/types/src/todo.tspackages/types/tsconfig.json
✅ Files skipped from review due to trivial changes (10)
- apps/api/package.json
- packages/kv/tsconfig.json
- packages/types/src/todo.ts
- packages/types/tsconfig.json
- packages/core/src/types/context.ts
- packages/types/package.json
- packages/kv/package.json
- packages/api/package.json
- packages/core/package.json
- apps/api/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/types/index.ts
- packages/core/src/index.ts
- packages/db/src/queries/todo.ts
- packages/core/src/services/todo.ts
Preview DeploymentCommit: |
Introduce a comprehensive set of features including Nano ID generation utilities, a Todo service with database integration, and a dashboard application. Enhance the API with TRPC support and improve UI components for better user experience.
Summary by CodeRabbit
New Features
Chores
Style